Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

version: do not append 0-hash to version strings from a clean tag #82

Merged
merged 1 commit into from
Dec 13, 2024

Conversation

nadiamoe
Copy link
Member

@nadiamoe nadiamoe commented Dec 9, 2024

Before this PR, the version script would always append -0-hash, even when on a clean tag. For example:

nadia@Nadiarch  ±8a885e5 ✓ 󱃾 curry-admin@Curry
16:18:08 ~/Devel/crocochrome $> git describe --dirty --tags --long
v0.3.1-0-g8a885e5

After this change, if run on a tagged commit without changes w.r.t. the tag, the output will just be the tag name:

nadia@Nadiarch  ±8a885e5 ✓ 󱃾 curry-admin@Curry
16:18:11 ~/Devel/crocochrome $> git describe --dirty --tags --always
v0.3.1

This short name should be sufficiently descriptive, and more semver-compliant.

@nadiamoe nadiamoe requested a review from a team as a code owner December 9, 2024 15:20
@ka3de
Copy link

ka3de commented Dec 11, 2024

My concern with this is that we use the same version script in all the repos, and that defines Docker image's tags etc, so I'm not against the change, but I think we should coordinate it globally so the "experience" across all repos is homogeneous.

@nadiamoe
Copy link
Member Author

@ka3de Should we try it here first and see if we like it, or causes any problem?
I think this repo in particular could benefit from this, in the form of not having to add versioning exceptions to repos that depend on it.

@nadiamoe nadiamoe merged commit 670a682 into main Dec 13, 2024
5 checks passed
@nadiamoe nadiamoe deleted the short-versions branch December 13, 2024 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants